Fast most_recent_only belief lookup for sensor statuses - #2463
Open
Ahmad-Wahid wants to merge 2 commits into
Open
Fast most_recent_only belief lookup for sensor statuses#2463Ahmad-Wahid wants to merge 2 commits into
Ahmad-Wahid wants to merge 2 commits into
Conversation
…g its beliefs The asset page reports, per sensor, how up to date each source type's data is. It got that by running one most_recent_only search per source type, for all seven default types, for every sensor on the page. Each of those is timely-beliefs' fast track, ORDER BY event_start DESC, belief_horizon ASC LIMIT 1, but with a join to data_source filtering on its type. That filter is the part no index on timed_belief can serve: PostgreSQL walks the sensor's events from the newest backwards, rechecking the type of each belief's source until it finds a match. Where a type recorded nothing for that sensor -- the common case, since most sensors have data from one or two types -- there is no match to find, so the scan reads every belief the sensor has before returning empty. The cost is therefore paid several times per sensor and grows with the data. The sensor_data_source summary (#2382) now answers which sources ever recorded for a sensor as a handful of rows. So resolve the sensor's sources once, group them by type, and name them in the belief query instead of filtering on their type. A type with no sources is skipped without a query at all, which is where the scans were, and a named source lets the reordered primary key (#2378), which leads with (sensor_id, source_id, event_start, belief_horizon), answer the LIMIT 1 as a backwards index scan reading a single row. Measured against a sensor carrying beliefs from one source, timing the whole per-sensor status lookup: at 300k beliefs 210 ms -> 5 ms, at 1M beliefs 611 ms -> 5 ms. The old path grows with the row count; the new one does not. Semantics are unchanged. The summary is a documented superset, so it can list a source whose beliefs have since been deleted -- costing one query that returns nothing, exactly as before -- but it cannot omit a source that has data. The source and exclude_source_types filters of the staleness search are applied to the source lookup instead of to the belief query, so they still hold. Tests pin that only the types that recorded are queried, that a second type is picked up, and that both source filters are honoured. The first, second and fourth fail against the old code with 7 queries where 1 or 2 are expected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
New/updated docstrings violate the repo’s line-break-after-punctuation convention, which needs to be fixed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR improves the performance of per-sensor “most recent belief” lookups used to compute sensor status (staleness) on the asset page by skipping source types that never recorded for a sensor and querying beliefs by explicit sources (leveraging the sensor_data_source summary table and the reordered PK).
Changes:
- Add
_sensor_sources_by_typeto resolve and group a sensor’s recorded sources (respectingsourceandexclude_source_typesfilters) before querying beliefs. - Update
_get_sensor_bdfs_by_source_typeto skip source types with no sources and passsource=[...]rather thansource_types=[...]intoTimedBelief.search. - Add targeted tests asserting the reduced query count and correct filter behavior, plus a changelog entry.
File summaries
| File | Description |
|---|---|
| flexmeasures/data/services/sensors.py | Resolve sources once via summary table and query most-recent beliefs by explicit sources, skipping empty types |
| flexmeasures/data/tests/test_sensor_status_queries.py | New tests validating query reduction and correctness of source / exclude_source_types filtering |
| documentation/changelog.rst | Changelog entry documenting the asset-page performance improvement |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Documentation build overview
|
Member
|
Nice, this comes in timely after #2382, getting a big benefit of that right away ❤️ |
Signed-off-by: Ahmad-Wahid <ahmedwahid16101@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2445.
The problem
The asset page shows, for each sensor, how up to date its data is per source type. To work that out, it ran one "most recent belief" query per source type — all seven of them — for every sensor on the page.
Each query asks for the newest belief whose source is of a given type. Nothing on the beliefs table can index that, so PostgreSQL walks the sensor's events from newest to oldest, checking each belief's source type until it finds a match. When a type recorded nothing for that sensor — which is the normal case, since most sensors have data from only one or two types — there is no match to find, so it reads every belief the sensor has before returning nothing.
So the page paid a full scan of a sensor's data five or six times per sensor, and it got slower as the data grew.
The fix
We now know which sources ever recorded for a sensor from the small
sensor_data_sourcesummary table (#2382) — a few rows, not a scan. So:Result
Timing the whole per-sensor status lookup, on a sensor with beliefs from one source:
The old version got slower as the sensor grew. The new one does not. An asset page multiplies these by its number of sensors.
Does it still give the same answers?
Yes.
The summary table is a superset: it can list a source whose beliefs have since been deleted, but it can never miss a source that has data. A stale entry only costs one query that comes back empty — exactly what happened before — so no status can change because of it.
The
sourceandexclude_source_typesfilters of a sensor's staleness search are now applied when looking up the sources rather than when querying the beliefs, so they still apply.Tests
Four new tests in
flexmeasures/data/tests/test_sensor_status_queries.pycheck that only the source types that actually recorded get queried, that a newly added type is picked up, and that both source filters are honoured. Three of them fail against the old code (7 queries where 1 or 2 are expected); the fourth guards the filter behaviour.Verified:
flexmeasures/ui(115 passed), the sensor/asset API tests (144 passed), and the relevant data tests (75 passed). Two unrelated pre-existing segfaults on my machine (the MILP solver intest_commitments.py, and thefakeredisjob tests) stop the fullflexmeasures/datarun; both crash the same way on an unmodifiedmain.🤖 Generated with Claude Code